Create pool redesign - feature branch #2537

kattylucy · 2024-11-11T11:08:27Z

Create pool redesign

https://www.figma.com/design/ng7qdNcSCXSDA6ZUdWIs6u/Pool-Overview%2F-Pool-Detail?node-id=4137-4180&node-type=section&m=dev

#2529

Approvals

Dev

github-actions · 2024-11-11T11:14:00Z

PR deployed in Google Cloud
URL: https://app-pr2537.k-f.dev
Commit #: 60e8383
To access the functions directly check the corresponding deploy Action

github-actions · 2024-11-11T11:14:04Z

PR deployed in Google Cloud
URL: https://pr2537-app-ff-production.k-f.dev
Commit #: 60e8383
To access the functions directly check the corresponding deploy Action

sophialittlejohn

Amazing to see this come together so nicely 🔥

There are a few comments from me and a few from the github bot that are probably worth fixing.

Here, I'm wondering what the wallet address is for this input? Would that be the pool admin?
If single is selected, I think we should hide the threshold section underneath.
The address (single and multisig) should be validated like the rest of the address inputs and show a red error message if it doesn't match

Here I think we should clean up the look of the box when there is an error message ;)

Lastly, I tried to create a pool and it failed with this error message:

Error: createType(Call):: Call: failed decoding poolRegistry.register:: Struct: failed on args: {"admin":"AccountId32","pool_id":"u64","tranche_inputs":"Vec<PalletPoolSystemTranchesTrancheInput>","currency":"{\"_enum\":{\"Native\":\"Null\",\"Tranche\":\"(u64,[u8;16])\",\"__Unused2\":\"Null\",\"AUSD\":\"Null\",\"ForeignAsset\":\"u32\",\"Staking\":\"CfgTypesTokensStakingCurrency\",\"LocalAsset\":\"u32\"}}","max_reserve":"u128","metadata":"Option<Bytes>","write_off_policy":"Vec<PalletLoansPolicyWriteOffRule>","pool_fees":"Vec<(CfgTraitsFeePoolFeeBucket,CfgTypesPoolsPoolFeeInfo)>"}:: Struct: failed on pool_fees: Vec<(CfgTraitsFeePoolFeeBucket,CfgTypesPoolsPoolFeeInfo)>:: Tuple: failed on 1:: Struct: failed on feeType: {"_enum":{"Fixed":"{\"limit\":\"CfgTypesPoolsPoolFeeAmount\"}","ChargedUpTo":"{\"limit\":\"CfgTypesPoolsPoolFeeAmount\"}"}}:: Cannot map Enum JSON, unable to find '' in fixed, chargedupto

I think it should be an easy fix but I'm happy to help you debug tomorrow!

centrifuge-app/src/pages/IssuerCreatePool/IssuerCategories.tsx

centrifuge-app/src/pages/IssuerCreatePool/PoolSetupSection.tsx

fabric/src/theme/tokens/colors.ts

sophialittlejohn · 2024-12-04T18:42:13Z

fabric/src/theme/tokens/theme.ts

@@ -17,7 +17,7 @@ const statusPromoteBg = '#f8107114'
 const colors = {
  textPrimary: grayScale[800],
  textSecondary: grayScale[500],
-  textDisabled: grayScale[300],
+  textDisabled: '#6A7280',


would it make sense to adjust a value in the grayScale instead of defining a new one?

sophialittlejohn · 2024-12-04T18:43:32Z

centrifuge-app/src/pages/IssuerCreatePool/index.tsx

        )
+
        const ratings = values.poolRatings.map((rating, index) => {


can be removed

onnovisser · 2024-12-10T09:33:42Z

centrifuge-app/src/components/FieldWithErrorMessage.tsx

+    form.setFieldValue(field.name, event.target.value)
+  }
+  return props.isUrl ? (
+    <URLInput


This seems like the wrong place for this. Also don't think it's needed. You should be able to use the URLInput as a prop to FieldWithErrorMessage:

<FieldWithErrorMessage name="website" as={URLInput} label="Website URL" placeholder="www.example.com" validate={validate.websiteNotRequired()} />

onnovisser · 2024-12-10T09:43:12Z

centrifuge-app/src/pages/IssuerCreatePool/index.tsx

        const [, , , , poolId] = args
        if (createType === 'immediate') {
-          setCreatedPoolId(poolId)
+          navigate(`/pools/${poolId}`)


I think this should stay in the useEffect like it was before. The data for the newly created pool won't always be immediately loaded, so it should wait for the data of the new pool before navigating to it.

onnovisser · 2024-12-10T09:49:46Z

centrifuge-app/src/pages/IssuerPool/Access/AssetOriginators.tsx

@@ -208,6 +208,7 @@ function AOForm({
          }),
          newMetadata ? cent.pools.setMetadata([poolId, newMetadata], { batch: true }) : of(null),
        ]).pipe(
+          // THIS IS WHERE PROXIES ARE BEING CREATED


can be removed

onnovisser · 2024-12-10T10:01:34Z

fabric/src/components/Checkbox/index.tsx

+  label,
+  errorMessage,
+  extendedClickArea,
+  variant = 'round',


This doesn't make sense, checkboxes should always be square. For single choice inputs, you can use RadioButton

onnovisser · 2024-12-11T09:05:52Z

Looking nice Katty! Just a few things I noticed

This button is misaligned when there's an error
Checkbox looks a bit misaligned too
Should there be a way to remove an address input here?
There's an ImageUpload component that may be better used for uploading images, instead of the FileUpload

The changed and currently used FileUpload component doesn't give a preview
Also the FileUpload component used to have a button to remove the file, which would be good to keep I think, because they aren't always required fields.
Now

Before
These placeholders have a different color

onnovisser · 2024-12-13T10:19:15Z

centrifuge-app/src/components/LoanList.tsx

@@ -279,6 +279,7 @@ export function LoanList({ loans, snapshots }: Props) {
                </Text>
              }
              onChange={(e) => setShowRepaid(!showRepaid)}
+              variant="square"


prop no longer needed

onnovisser · 2024-12-13T10:20:08Z

centrifuge-app/src/components/Menu/IssuerMenu.tsx

@@ -56,9 +62,9 @@ export function IssuerMenu({ defaultOpen = false, children }: IssuerMenuProps) {
        Issuer
        {isLarge &&
          (open ? (
-            <IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} />
+            <IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} color="white" />


Suggested change

<IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} color="white" />

<IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} color="textInverted" />

onnovisser · 2024-12-13T10:25:56Z

centrifuge-app/src/pages/IssuerCreatePool/index.tsx

                ),
                multisigAddr
                  ? api.tx.multisig.asMulti(adminMultisig.threshold, otherMultisigSigners, null, proxiedPoolCreate, 0)
                  : proxiedPoolCreate,
              ].filter(Boolean)
            )
            setMultisigData({ callData: proxiedPoolCreate.method.toHex(), hash: proxiedPoolCreate.method.hash.toHex() })
-            return cent.wrapSignAndSend(api, submittable, { ...options, multisig: undefined, proxies: undefined })
+            return cent.wrapSignAndSend(api, submittable, { ...options })


why was multisig: undefined, proxies: undefined removed? That should likely remain

onnovisser · 2024-12-13T10:36:12Z

centrifuge-app/src/pages/IssuerCreatePool/utils.ts

+}
+
+export const pinFileIfExists = async (centrifuge: ReturnType<typeof useCentrifuge>, file: File | null) =>
+  file ? pinFile(centrifuge, file) : Promise.resolve(null)


Suggested change

file ? pinFile(centrifuge, file) : Promise.resolve(null)

file ? pinFile(centrifuge, file) : null

onnovisser · 2024-12-13T10:36:42Z

centrifuge-app/src/pages/IssuerCreatePool/utils.ts

+  return { uri: pinned.uri, mime: file.type }
+}
+
+export const pinFileIfExists = async (centrifuge: ReturnType<typeof useCentrifuge>, file: File | null) =>


the Centrifuge type can be imported from CentrifugeJS

onnovisser · 2024-12-13T10:44:01Z

centrifuge-app/src/pages/Pool/Assets/index.tsx

-  const { data: loans, isLoading } = useLoans(poolId)
-  const { isLoading: isLoadingSnapshots, data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toString())
+  const { data: loans } = useLoans(poolId)
+  const { data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toString())


Suggested change

const { data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toString())

const { data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toISOString().slice(0, 10))

Now this will fetch every render because the date will be different every time

onnovisser · 2024-12-13T10:46:09Z

centrifuge-js/src/modules/pools.ts

    ],
    options?: TransactionOptions
  ) {
    const [admin, poolId, tranches, currency, maxReserve, metadata, fees] = args
-    const trancheInput = tranches.map((t, i) => ({


Doesn't seem like the right move to move this responsibility to the client. Was there a reason for this?

It was Sophia's previous feedback. She said it looked messy to have logic in both places, create pool and pool.ts file.

onnovisser · 2024-12-13T10:46:55Z

fabric/src/components/FileUpload/index.tsx

-const Spinner = styled(IconSpinner)`
-  animation: ${rotate} 600ms linear infinite;
-`
+// const Spinner = styled(IconSpinner)`


can just be removed if no longer needed

onnovisser · 2024-12-13T10:52:57Z

fabric/src/components/Select/index.tsx

@@ -18,6 +18,7 @@ export type SelectProps = React.SelectHTMLAttributes<HTMLSelectElement> & {
  errorMessage?: string
  small?: boolean
  hideBorder?: boolean
+  activePlaceholder?: boolean


what does activePlaceholder mean? If it's not required to select an option, shouldn't there just be a None option?

* Add pool structure UI changes * Small UI fix * Avoid deletion on entries if less than one * Add logic to single / multi sign * Fix linter errors

* Fix ts error and change logic for onboarding values * Add create pool existing functionality * Cleanup types * cleanup * Add deposit banner * Fix linter errors * Add metadata values * Cleanup types * Add onboarding functionality and UI fixes * Add proxies functionality * Fix ts errors * Add create pool dialog * Add dialogs * Add review feedback * wip * Add waiting before redirecting to avoid error * Remove default empty pool fee

* Bug fixes and add proposal link * Fix ratings creating empty value

Co-authored-by: Sophia <[email protected]>

APY should be based on junior token

kattylucy force-pushed the create_pool_redesign branch 4 times, most recently from c962a38 to 4d6c428 Compare November 15, 2024 11:18

kattylucy force-pushed the create_pool_redesign branch 2 times, most recently from 45db2a9 to 19988cc Compare November 21, 2024 09:56

kattylucy changed the title ~~Create pool redesign~~ Create pool redesign - feature branch Nov 21, 2024

kattylucy requested review from onnovisser and sophialittlejohn December 4, 2024 11:45

kattylucy marked this pull request as ready for review December 4, 2024 11:45

sophialittlejohn requested changes Dec 4, 2024

View reviewed changes

onnovisser reviewed Dec 10, 2024

View reviewed changes

kattylucy requested a review from onnovisser December 10, 2024 14:39

kattylucy force-pushed the create_pool_redesign branch from 2f39697 to 17d3fde Compare December 12, 2024 09:42

onnovisser reviewed Dec 13, 2024

View reviewed changes

Fabric changes

5a8d248

kattylucy and others added 24 commits December 13, 2024 12:18

Type updates

7f2ddbf

Add new create pool index

f0d705c

Add pool structure section

aa7fab0

Fix sidebar

135c08d

Add pool details section

6ce1d06

Add pool setup section

25e47ff

Minor UI fix

93dc2cf

Pool structure - create pool (#2543)

dfbd6ae

* Add pool structure UI changes * Small UI fix * Avoid deletion on entries if less than one * Add logic to single / multi sign * Fix linter errors

Create pool bugs and fixes (#2550)

46c054d

* Bug fixes and add proposal link * Fix ratings creating empty value

Update fabric/src/theme/tokens/colors.ts

e8eb5fa

Co-authored-by: Sophia <[email protected]>

Update centrifuge-app/src/pages/IssuerCreatePool/IssuerCategories.tsx

9a67f06

Co-authored-by: Sophia <[email protected]>

Update centrifuge-app/src/pages/IssuerCreatePool/PoolSetupSection.tsx

8993f7c

Co-authored-by: Sophia <[email protected]>

Add feedback

2ee76df

feedback

9bd7e31

Feedback changes

171f9cf

Add feeback

24e0378

Fix tranches apy

f353ff6

APY should be based on junior token

Add feeback

44e869b

Fix placeholder color

9276d7a

Replace file upload with image upload

4fb7691

Bug fixes

a2484df

Code review feedback

bc1c277

Add jay's feedback and cleanup

60e8383

kattylucy force-pushed the create_pool_redesign branch from ee0c6d8 to 60e8383 Compare December 13, 2024 11:36

kattylucy requested a review from onnovisser December 13, 2024 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create pool redesign - feature branch #2537

Create pool redesign - feature branch #2537

kattylucy commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

sophialittlejohn left a comment

sophialittlejohn Dec 4, 2024

sophialittlejohn Dec 4, 2024

onnovisser Dec 10, 2024

onnovisser Dec 10, 2024

onnovisser Dec 10, 2024

onnovisser Dec 10, 2024 •

edited

Loading

onnovisser commented Dec 11, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

onnovisser Dec 13, 2024

kattylucy Dec 13, 2024

onnovisser Dec 13, 2024 •

edited

Loading

onnovisser Dec 13, 2024

		)

		const ratings = values.poolRatings.map((rating, index) => {

	<IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} color="white" />
	<IconChevronDown size={['iconMedium', 'iconMedium', 'iconSmall']} color="textInverted" />

	file ? pinFile(centrifuge, file) : Promise.resolve(null)
	file ? pinFile(centrifuge, file) : null

	const { data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toString())
	const { data: snapshots } = useAllPoolAssetSnapshots(poolId, new Date().toISOString().slice(0, 10))

Create pool redesign - feature branch #2537

Are you sure you want to change the base?

Create pool redesign - feature branch #2537

Conversation

kattylucy commented Nov 11, 2024 • edited Loading

Approvals

github-actions bot commented Nov 11, 2024 • edited Loading

github-actions bot commented Nov 11, 2024 • edited Loading

sophialittlejohn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onnovisser Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

onnovisser commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onnovisser Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kattylucy commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

onnovisser Dec 10, 2024 •

edited

Loading

onnovisser Dec 13, 2024 •

edited

Loading